Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Block Supports: Add border color, style and width support #30124

Merged
merged 13 commits into from Apr 19, 2021

Conversation

aaronrobertshaw
Copy link
Contributor

@aaronrobertshaw aaronrobertshaw commented Mar 23, 2021

Fixes: #29616

Description

This PR extends the border related block support to include color, style and width options.

These border options are disabled by default.

How has this been tested?

Manually with a couple of small updates to existing unit tests.

Testing Instructions:

  1. Enable custom border color, radius, style and width in your theme.json file.
  2. Create a new post and add a group block to it
  3. Select the group block and confirm the "Border settings" panel displays in the sidebar
  4. Expand the "Border settings" panel and try editing each of the border properties.
  5. Confirm the block updates as expected in the editor, save and then confirm correct classes and styles on the frontend.
  6. Open the Site Editor then its global styles sidebar. Switch to the "By block type" tab and expand the Group block panel.
  7. Confirm that the border support features you have enabled are present.
  8. Test changing the global styles for the group block's borders.

Unit Tests:

  • npm run test-unit:watch packages/block-editor/src/hooks/test/style.js
  • wp-env run phpunit "phpunit -c /var/www/html/wp-content/plugins/gutenberg/phpunit.xml.dist /var/www/html/wp-content/plugins/gutenberg/phpunit/class-wp-theme-json-test.php"

Screenshots

Types of changes

New feature

Next steps

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@aaronrobertshaw aaronrobertshaw added [Block] Group Affects the Group Block [Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi labels Mar 23, 2021
@aaronrobertshaw aaronrobertshaw self-assigned this Mar 23, 2021
@aaronrobertshaw
Copy link
Contributor Author

@nosolosw I've made a start on adding the extra properties to the border block support. It would be great if you could spare a few moments to sanity check the approach taken for the border colors. Thanks in advance!

@github-actions
Copy link

github-actions bot commented Mar 23, 2021

Size Change: +37.3 kB (+3%)

Total Size: 1.46 MB

Filename Size Change
build/annotations/index.js 3.78 kB -9 B (0%)
build/api-fetch/index.js 3.41 kB -1 B (0%)
build/autop/index.js 2.81 kB -10 B (0%)
build/blob/index.js 664 B -1 B (0%)
build/block-directory/index.js 8.62 kB -6 B (0%)
build/block-editor/index.js 129 kB +2.24 kB (+2%)
build/block-editor/style-rtl.css 12.5 kB +134 B (+1%)
build/block-editor/style.css 12.5 kB +132 B (+1%)
build/block-library/blocks/buttons/style-rtl.css 368 B +4 B (+1%)
build/block-library/blocks/buttons/style.css 368 B +5 B (+1%)
build/block-library/blocks/navigation/style-rtl.css 272 B +68 B (+33%) 🚨
build/block-library/blocks/navigation/style.css 271 B +66 B (+32%) 🚨
build/block-library/blocks/query/editor-rtl.css 810 B +15 B (+2%)
build/block-library/blocks/query/editor.css 809 B +15 B (+2%)
build/block-library/blocks/site-logo/editor-rtl.css 440 B +2 B (0%)
build/block-library/blocks/site-logo/editor.css 441 B +3 B (+1%)
build/block-library/blocks/site-logo/style-rtl.css 154 B +4 B (+3%)
build/block-library/blocks/site-logo/style.css 154 B +4 B (+3%)
build/block-library/blocks/social-links/editor-rtl.css 796 B +20 B (+3%)
build/block-library/blocks/social-links/editor.css 795 B +19 B (+2%)
build/block-library/blocks/verse/editor-rtl.css 0 B -50 B (removed) 🏆
build/block-library/blocks/verse/editor.css 0 B -50 B (removed) 🏆
build/block-library/blocks/video/editor-rtl.css 568 B +64 B (+13%) ⚠️
build/block-library/blocks/video/editor.css 569 B +66 B (+13%) ⚠️
build/block-library/editor-rtl.css 9.79 kB +27 B (0%)
build/block-library/editor.css 9.78 kB +29 B (0%)
build/block-library/index.js 153 kB +290 B (0%)
build/block-library/style-rtl.css 9.43 kB +48 B (+1%)
build/block-library/style.css 9.44 kB +50 B (+1%)
build/block-serialization-default-parser/index.js 1.87 kB +1 B (0%)
build/blocks/index.js 48.5 kB +63 B (0%)
build/components/index.js 286 kB +67 B (0%)
build/components/style-rtl.css 16.3 kB -49 B (0%)
build/components/style.css 16.3 kB -49 B (0%)
build/compose/index.js 11.3 kB +5 B (0%)
build/core-data/index.js 17.1 kB -7 B (0%)
build/customize-widgets/index.js 7.11 kB +17 B (0%)
build/data-controls/index.js 835 B -4 B (0%)
build/data/index.js 8.87 kB -7 B (0%)
build/date/index.js 31.9 kB -1 B (0%)
build/dom-ready/index.js 577 B +1 B (0%)
build/dom/index.js 5.1 kB -89 B (-2%)
build/edit-navigation/index.js 17 kB -15 B (0%)
build/edit-post/index.js 339 kB +31.7 kB (+10%) ⚠️
build/edit-post/style-rtl.css 6.99 kB +51 B (+1%)
build/edit-post/style.css 6.98 kB +51 B (+1%)
build/edit-site/index.js 28.6 kB +618 B (+2%)
build/edit-site/style-rtl.css 4.9 kB +289 B (+6%) 🔍
build/edit-site/style.css 4.89 kB +286 B (+6%) 🔍
build/edit-widgets/index.js 16.8 kB +1.1 kB (+7%) 🔍
build/editor/index.js 42.6 kB +203 B (0%)
build/element/index.js 4.61 kB -10 B (0%)
build/format-library/index.js 6.75 kB -10 B (0%)
build/html-entities/index.js 622 B -1 B (0%)
build/i18n/index.js 4.04 kB +27 B (+1%)
build/keyboard-shortcuts/index.js 2.52 kB -9 B (0%)
build/keycodes/index.js 1.95 kB -8 B (0%)
build/list-reusable-blocks/index.js 3.19 kB +1 B (0%)
build/media-utils/index.js 5.38 kB -2 B (0%)
build/notices/index.js 1.85 kB -5 B (0%)
build/nux/index.js 3.41 kB -9 B (0%)
build/primitives/index.js 1.42 kB -7 B (0%)
build/priority-queue/index.js 791 B +1 B (0%)
build/redux-routine/index.js 2.83 kB -3 B (0%)
build/reusable-blocks/index.js 3.78 kB -8 B (0%)
build/rich-text/index.js 13.5 kB -7 B (0%)
build/server-side-render/index.js 2.6 kB -6 B (0%)
build/shortcode/index.js 1.7 kB +2 B (0%)
build/url/index.js 3.02 kB -9 B (0%)
build/wordcount/index.js 1.22 kB +1 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/block-directory/style-rtl.css 1 kB 0 B
build/block-directory/style.css 1.01 kB 0 B
build/block-library/blocks/archives/editor-rtl.css 61 B 0 B
build/block-library/blocks/archives/editor.css 60 B 0 B
build/block-library/blocks/audio/editor-rtl.css 58 B 0 B
build/block-library/blocks/audio/editor.css 58 B 0 B
build/block-library/blocks/audio/style-rtl.css 112 B 0 B
build/block-library/blocks/audio/style.css 112 B 0 B
build/block-library/blocks/block/editor-rtl.css 161 B 0 B
build/block-library/blocks/block/editor.css 161 B 0 B
build/block-library/blocks/button/editor-rtl.css 475 B 0 B
build/block-library/blocks/button/editor.css 474 B 0 B
build/block-library/blocks/button/style-rtl.css 503 B 0 B
build/block-library/blocks/button/style.css 503 B 0 B
build/block-library/blocks/buttons/editor-rtl.css 315 B 0 B
build/block-library/blocks/buttons/editor.css 315 B 0 B
build/block-library/blocks/calendar/style-rtl.css 208 B 0 B
build/block-library/blocks/calendar/style.css 208 B 0 B
build/block-library/blocks/categories/editor-rtl.css 84 B 0 B
build/block-library/blocks/categories/editor.css 83 B 0 B
build/block-library/blocks/categories/style-rtl.css 79 B 0 B
build/block-library/blocks/categories/style.css 79 B 0 B
build/block-library/blocks/code/style-rtl.css 90 B 0 B
build/block-library/blocks/code/style.css 90 B 0 B
build/block-library/blocks/columns/editor-rtl.css 190 B 0 B
build/block-library/blocks/columns/editor.css 190 B 0 B
build/block-library/blocks/columns/style-rtl.css 436 B 0 B
build/block-library/blocks/columns/style.css 435 B 0 B
build/block-library/blocks/cover/editor-rtl.css 605 B 0 B
build/block-library/blocks/cover/editor.css 605 B 0 B
build/block-library/blocks/cover/style-rtl.css 1.23 kB 0 B
build/block-library/blocks/cover/style.css 1.23 kB 0 B
build/block-library/blocks/embed/editor-rtl.css 486 B 0 B
build/block-library/blocks/embed/editor.css 486 B 0 B
build/block-library/blocks/embed/style-rtl.css 401 B 0 B
build/block-library/blocks/embed/style.css 400 B 0 B
build/block-library/blocks/file/editor-rtl.css 175 B 0 B
build/block-library/blocks/file/editor.css 174 B 0 B
build/block-library/blocks/file/style-rtl.css 248 B 0 B
build/block-library/blocks/file/style.css 248 B 0 B
build/block-library/blocks/freeform/editor-rtl.css 2.44 kB 0 B
build/block-library/blocks/freeform/editor.css 2.44 kB 0 B
build/block-library/blocks/gallery/editor-rtl.css 704 B 0 B
build/block-library/blocks/gallery/editor.css 705 B 0 B
build/block-library/blocks/gallery/style-rtl.css 1.09 kB 0 B
build/block-library/blocks/gallery/style.css 1.09 kB 0 B
build/block-library/blocks/group/editor-rtl.css 160 B 0 B
build/block-library/blocks/group/editor.css 160 B 0 B
build/block-library/blocks/group/style-rtl.css 57 B 0 B
build/block-library/blocks/group/style.css 57 B 0 B
build/block-library/blocks/heading/editor-rtl.css 129 B 0 B
build/block-library/blocks/heading/editor.css 129 B 0 B
build/block-library/blocks/heading/style-rtl.css 76 B 0 B
build/block-library/blocks/heading/style.css 76 B 0 B
build/block-library/blocks/html/editor-rtl.css 281 B 0 B
build/block-library/blocks/html/editor.css 281 B 0 B
build/block-library/blocks/image/editor-rtl.css 717 B 0 B
build/block-library/blocks/image/editor.css 716 B 0 B
build/block-library/blocks/image/style-rtl.css 476 B 0 B
build/block-library/blocks/image/style.css 478 B 0 B
build/block-library/blocks/latest-comments/style-rtl.css 281 B 0 B
build/block-library/blocks/latest-comments/style.css 282 B 0 B
build/block-library/blocks/latest-posts/editor-rtl.css 137 B 0 B
build/block-library/blocks/latest-posts/editor.css 137 B 0 B
build/block-library/blocks/latest-posts/style-rtl.css 523 B 0 B
build/block-library/blocks/latest-posts/style.css 522 B 0 B
build/block-library/blocks/legacy-widget/editor-rtl.css 398 B 0 B
build/block-library/blocks/legacy-widget/editor.css 399 B 0 B
build/block-library/blocks/list/style-rtl.css 63 B 0 B
build/block-library/blocks/list/style.css 63 B 0 B
build/block-library/blocks/media-text/editor-rtl.css 191 B 0 B
build/block-library/blocks/media-text/editor.css 191 B 0 B
build/block-library/blocks/media-text/style-rtl.css 535 B 0 B
build/block-library/blocks/media-text/style.css 532 B 0 B
build/block-library/blocks/more/editor-rtl.css 434 B 0 B
build/block-library/blocks/more/editor.css 434 B 0 B
build/block-library/blocks/navigation-link/editor-rtl.css 597 B 0 B
build/block-library/blocks/navigation-link/editor.css 597 B 0 B
build/block-library/blocks/navigation-link/style-rtl.css 1.07 kB 0 B
build/block-library/blocks/navigation-link/style.css 1.07 kB 0 B
build/block-library/blocks/navigation/editor-rtl.css 1.24 kB 0 B
build/block-library/blocks/navigation/editor.css 1.24 kB 0 B
build/block-library/blocks/nextpage/editor-rtl.css 395 B 0 B
build/block-library/blocks/nextpage/editor.css 395 B 0 B
build/block-library/blocks/page-list/editor-rtl.css 239 B 0 B
build/block-library/blocks/page-list/editor.css 240 B 0 B
build/block-library/blocks/page-list/style-rtl.css 167 B 0 B
build/block-library/blocks/page-list/style.css 167 B 0 B
build/block-library/blocks/paragraph/editor-rtl.css 157 B 0 B
build/block-library/blocks/paragraph/editor.css 157 B 0 B
build/block-library/blocks/paragraph/style-rtl.css 247 B 0 B
build/block-library/blocks/paragraph/style.css 248 B 0 B
build/block-library/blocks/post-author/editor-rtl.css 209 B 0 B
build/block-library/blocks/post-author/editor.css 209 B 0 B
build/block-library/blocks/post-author/style-rtl.css 183 B 0 B
build/block-library/blocks/post-author/style.css 184 B 0 B
build/block-library/blocks/post-comments-form/style-rtl.css 250 B 0 B
build/block-library/blocks/post-comments-form/style.css 250 B 0 B
build/block-library/blocks/post-content/editor-rtl.css 139 B 0 B
build/block-library/blocks/post-content/editor.css 139 B 0 B
build/block-library/blocks/post-excerpt/editor-rtl.css 73 B 0 B
build/block-library/blocks/post-excerpt/editor.css 73 B 0 B
build/block-library/blocks/post-excerpt/style-rtl.css 69 B 0 B
build/block-library/blocks/post-excerpt/style.css 69 B 0 B
build/block-library/blocks/post-featured-image/editor-rtl.css 338 B 0 B
build/block-library/blocks/post-featured-image/editor.css 338 B 0 B
build/block-library/blocks/post-featured-image/style-rtl.css 100 B 0 B
build/block-library/blocks/post-featured-image/style.css 100 B 0 B
build/block-library/blocks/post-title/style-rtl.css 60 B 0 B
build/block-library/blocks/post-title/style.css 60 B 0 B
build/block-library/blocks/preformatted/style-rtl.css 103 B 0 B
build/block-library/blocks/preformatted/style.css 103 B 0 B
build/block-library/blocks/pullquote/editor-rtl.css 183 B 0 B
build/block-library/blocks/pullquote/editor.css 183 B 0 B
build/block-library/blocks/pullquote/style-rtl.css 318 B 0 B
build/block-library/blocks/pullquote/style.css 318 B 0 B
build/block-library/blocks/query-loop/editor-rtl.css 83 B 0 B
build/block-library/blocks/query-loop/editor.css 82 B 0 B
build/block-library/blocks/query-loop/style-rtl.css 315 B 0 B
build/block-library/blocks/query-loop/style.css 317 B 0 B
build/block-library/blocks/query-pagination-numbers/editor-rtl.css 122 B 0 B
build/block-library/blocks/query-pagination-numbers/editor.css 121 B 0 B
build/block-library/blocks/query-pagination/editor-rtl.css 270 B 0 B
build/block-library/blocks/query-pagination/editor.css 262 B 0 B
build/block-library/blocks/query-pagination/style-rtl.css 168 B 0 B
build/block-library/blocks/query-pagination/style.css 168 B 0 B
build/block-library/blocks/query-title/editor-rtl.css 86 B 0 B
build/block-library/blocks/query-title/editor.css 86 B 0 B
build/block-library/blocks/quote/style-rtl.css 169 B 0 B
build/block-library/blocks/quote/style.css 169 B 0 B
build/block-library/blocks/rss/editor-rtl.css 201 B 0 B
build/block-library/blocks/rss/editor.css 202 B 0 B
build/block-library/blocks/rss/style-rtl.css 290 B 0 B
build/block-library/blocks/rss/style.css 290 B 0 B
build/block-library/blocks/search/editor-rtl.css 189 B 0 B
build/block-library/blocks/search/editor.css 189 B 0 B
build/block-library/blocks/search/style-rtl.css 359 B 0 B
build/block-library/blocks/search/style.css 362 B 0 B
build/block-library/blocks/separator/editor-rtl.css 99 B 0 B
build/block-library/blocks/separator/editor.css 99 B 0 B
build/block-library/blocks/separator/style-rtl.css 251 B 0 B
build/block-library/blocks/separator/style.css 251 B 0 B
build/block-library/blocks/shortcode/editor-rtl.css 512 B 0 B
build/block-library/blocks/shortcode/editor.css 512 B 0 B
build/block-library/blocks/social-link/editor-rtl.css 164 B 0 B
build/block-library/blocks/social-link/editor.css 165 B 0 B
build/block-library/blocks/social-links/style-rtl.css 1.32 kB 0 B
build/block-library/blocks/social-links/style.css 1.33 kB 0 B
build/block-library/blocks/spacer/editor-rtl.css 308 B 0 B
build/block-library/blocks/spacer/editor.css 308 B 0 B
build/block-library/blocks/spacer/style-rtl.css 48 B 0 B
build/block-library/blocks/spacer/style.css 48 B 0 B
build/block-library/blocks/table/editor-rtl.css 478 B 0 B
build/block-library/blocks/table/editor.css 478 B 0 B
build/block-library/blocks/table/style-rtl.css 402 B 0 B
build/block-library/blocks/table/style.css 402 B 0 B
build/block-library/blocks/tag-cloud/editor-rtl.css 118 B 0 B
build/block-library/blocks/tag-cloud/editor.css 118 B 0 B
build/block-library/blocks/tag-cloud/style-rtl.css 94 B 0 B
build/block-library/blocks/tag-cloud/style.css 94 B 0 B
build/block-library/blocks/template-part/editor-rtl.css 552 B 0 B
build/block-library/blocks/template-part/editor.css 551 B 0 B
build/block-library/blocks/term-description/editor-rtl.css 90 B 0 B
build/block-library/blocks/term-description/editor.css 90 B 0 B
build/block-library/blocks/text-columns/editor-rtl.css 95 B 0 B
build/block-library/blocks/text-columns/editor.css 95 B 0 B
build/block-library/blocks/text-columns/style-rtl.css 166 B 0 B
build/block-library/blocks/text-columns/style.css 166 B 0 B
build/block-library/blocks/verse/style-rtl.css 87 B 0 B
build/block-library/blocks/verse/style.css 87 B 0 B
build/block-library/blocks/video/style-rtl.css 173 B 0 B
build/block-library/blocks/video/style.css 173 B 0 B
build/block-library/common-rtl.css 1.31 kB 0 B
build/block-library/common.css 1.31 kB 0 B
build/block-library/reset-rtl.css 502 B 0 B
build/block-library/reset.css 503 B 0 B
build/block-library/theme-rtl.css 692 B 0 B
build/block-library/theme.css 693 B 0 B
build/block-serialization-spec-parser/index.js 3.06 kB 0 B
build/customize-widgets/style-rtl.css 630 B 0 B
build/customize-widgets/style.css 631 B 0 B
build/deprecated/index.js 787 B 0 B
build/edit-navigation/style-rtl.css 2.86 kB 0 B
build/edit-navigation/style.css 2.86 kB 0 B
build/edit-post/classic-rtl.css 454 B 0 B
build/edit-post/classic.css 454 B 0 B
build/edit-widgets/style-rtl.css 2.97 kB 0 B
build/edit-widgets/style.css 2.98 kB 0 B
build/editor/style-rtl.css 3.92 kB 0 B
build/editor/style.css 3.92 kB 0 B
build/escape-html/index.js 735 B 0 B
build/format-library/style-rtl.css 637 B 0 B
build/format-library/style.css 639 B 0 B
build/hooks/index.js 2.28 kB 0 B
build/is-shallow-equal/index.js 699 B 0 B
build/list-reusable-blocks/style-rtl.css 629 B 0 B
build/list-reusable-blocks/style.css 628 B 0 B
build/nux/style-rtl.css 731 B 0 B
build/nux/style.css 727 B 0 B
build/plugins/index.js 2.95 kB 0 B
build/react-i18n/index.js 1.46 kB 0 B
build/reusable-blocks/style-rtl.css 225 B 0 B
build/reusable-blocks/style.css 225 B 0 B
build/token-list/index.js 1.27 kB 0 B
build/viewport/index.js 1.86 kB 0 B
build/warning/index.js 1.14 kB 0 B

compressed-size-action

@gziolo gziolo added the Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json label Mar 23, 2021
@aaronrobertshaw
Copy link
Contributor Author

Thanks for the time and effort on the review @grzim. Appreciate it 👍

@aaronrobertshaw
Copy link
Contributor Author

I've rebased this PR and added skipping serialization of border support properties. There have also been a few inline comments added to help clarify the intent of the code.

Copy link
Contributor

@stacimc stacimc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This tests well for me in both the editor and on the frontend, with various permutations of the available settings. I also tested enabling only a subset of the color/style/width/radius options in the theme.json, omitting the color support for example.

LGTM, this would be a great addition 🎉

Copy link
Contributor

@apeatling apeatling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking really good, thanks for your work on this one. Tested as per instructions and it worked well. ♥️

@aaronrobertshaw
Copy link
Contributor Author

A new border panel has been added to the Global Styles sidebar and the PRs testing instructions updated. This side of the changes could do with closer inspection and testing.

...style,
border: {
...style?.border,
color: colorObject?.slug ? undefined : value,
Copy link
Member

@ramonjd ramonjd Apr 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm just testing this PR and, for me, the border color panel doesn't add the border color inline style to the root element. The has-{color}-border-color class is present however.

The result is that the border color doesn't change when I select a new border color.

Strangely, changing the text color modifies the border color, though I can't see why in the computed styles.

color-undefined

If I change this line to color: colorObject?.color || value, it works as I'd expect.

Is there a reason why the style prop should be undefined here in the presence of colorObject?.slug?

Or are there CSS definitions for .has-{color}-border-color classes that I'm not seeing (or something else I'm missing)?

Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a good chance I missed something here so I'll look into it. Thanks for testing.

Is there a reason why the style prop should be undefined here?

This was undefined so that when using a named color it uses the CSS classes generated for the color from the current palette. If it stored the color value there the inline style overrules it. This could become a problem if the theme changes the color palette or the user switches themes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strangely, changing the text color modifies the border color, though I can't see why in the computed styles.

I have the suspicion that the theme sets the border color to the currentColor which would be determined by the text color. The inline border style works for me when a border style is set though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was undefined so that when using a named color it uses the CSS classes generated for the color from the current palette.

Thanks, that's what I suspected, was just scratching my head as to where those CSS class definitions were being (or should be) generated.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the clarification. Just dropping here to add that I had the TwentyTwentyOne theme activated.

I could see the expected behaviour after switching to TT1 Blocks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nosolosw would you be able to point me in the right direction for getting the color styles generated from the theme.json color palette available for themes such as TwentyTwenty?

I believe the active theme currently has to satisfy this check before the necessary styles are included.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useEditorFeature( 'color.palette' ) will have the color palette, whether or not the theme has support for theme.json

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue I'm trying to solve here is some older themes don't qualify to have the named color CSS classes included. In other words they are missing has-<color>-border-color.

If the TT1 Blocks or TwentyTwenty Blocks themes are active, they get the generated color classes fine. Something older such as the TwentyTwenty (non block based) theme doesn't get these so while the block has the correct CSS class on the frontend the CSS class doesn't exist in the stylesheet.

Copy link
Member

@oandregal oandregal Apr 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see. Themes that don't support theme.json won't have the preset classes automatically enqueued for them, they add them themselves. At some point in the past, we experimented with enqueuing the classes for these themes as well but heard reports that this wasn't expected, so we stopped doing it.

What I see is that named border-color classes are properly attached to the block (for any theme), and that's the only thing relevant for the hook. In terms of the global stylesheet, I also see them generated and enqueued in the embedded stylesheet global-styles-inline-css for themes with theme.json support, so everything is working as expected.

One thing we should do is to add this as a dev note saying: there's this new property so themes need to enqueue the classes for the border-colors as well. Only if this feature becomes stable for 5.8. I'll add the label so we can find them later.

Also ccing a few theme people FYI (re: has-<color>-border-color classes) ― @carolinan @aristath @scruffian @MaggieCabrera

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation. That helps my understanding 👍

@ramonjd ramonjd mentioned this pull request Apr 14, 2021
7 tasks
@oandregal
Copy link
Member

Going to add the dev note label as per this conversation https://github.com/WordPress/gutenberg/pull/30124/files#r614248792

@oandregal oandregal added the Needs Dev Note Requires a developer note for a major WordPress release cycle label Apr 15, 2021
Copy link
Member

@oandregal oandregal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've left a note that I'd like addressed at https://github.com/WordPress/gutenberg/pull/30124/files#r614149424 Other than that it's working nicely and the code is ready.

Great work here. Thanks for your patience and persistence!

@aaronrobertshaw
Copy link
Contributor Author

I've left a note that I'd like addressed at https://github.com/WordPress/gutenberg/pull/30124/files#r614149424 Other than that it's working nicely and the code is ready.

I've fixed up the support check code as requested. It should be pretty close now 🚢

Great work here. Thanks for your patience and persistence!

Team effort! I appreciate all the time you've spent reviewing this and other PRs 🙇

@aaronrobertshaw aaronrobertshaw merged commit 933c45a into trunk Apr 19, 2021
@aaronrobertshaw aaronrobertshaw deleted the update/border-block-support branch April 19, 2021 03:50
@github-actions github-actions bot added this to the Gutenberg 10.5 milestone Apr 19, 2021
@oandregal
Copy link
Member

oandregal commented Apr 21, 2021

Added missed docs for these settings at #31039 (updated: also for styles #31040 )

@aaronrobertshaw
Copy link
Contributor Author

Added another PR for updating the block-supports.md reference guide - #31069

@oandregal
Copy link
Member

We forgot to register the border color preset in the client as well, so the preset classes are not being generated in the site editor. Fix for this at #31217

@youknowriad
Copy link
Contributor

I remove the dev note label because it's still experimental we should add the label to the PR that make this stable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Group Affects the Group Block [Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for border properties
9 participants